feat: switch to yeoman-env - #71
Conversation
| "camelcase-keys": "^4.0.0", | ||
| "debug": "^2.6.1", | ||
| "generator-loopback": "^5.5.1", | ||
| "generator-loopback": "file:../generator-loopback", |
There was a problem hiding this comment.
temporarily points to a local module. This PR will merge after we release a new major version for generator-loopback.
There was a problem hiding this comment.
Looks reasonable to me 👍
Please run the CLI manually to check that we are correctly setting up options and arguments, there have been issues with this integration point in the past.
Things to try:
lb app foobar- picksfoobaras the app namelb model --bluemix- understands thatbluemixoption was set and returns error "datasources-config.json not found"
|
|
||
| debug('env.run %j %j', args, options); | ||
| env.run(args, options); | ||
| env.lookup(function() { |
There was a problem hiding this comment.
Is env.lookup allowed to fail? Should we add error-handling code to the callback? I am not familiar with this Yeoman API, just asking.
env.lookup(function(err) {
if (err) {
// handle the error and exit (?)
}
debug('changing directory back to %s', cwd);
// etc.
}There was a problem hiding this comment.
Good point, while the implementation of lookup(cb) always returns cb(null), see code. It looks like a problem of yeoman-environment, it doesn't propagate the error got from this._tryRegistering.
Anyway I am adding the err check :)
|
@bajtos Thanks for the review and suggestion!
Sure 👍The PR is written when I invoke the generator by |
0c318bd to
d5d166f
Compare
jannyHou
changed the title
| { | ||
| "name": "loopback-cli", | ||
| "version": "4.2.0", | ||
| "version": "5.0.0", |
There was a problem hiding this comment.
Is this to prevent tests breaking downstream?
There was a problem hiding this comment.
sorry for replying late 🙈
prevent tests breaking downstream
Exactly.
| const lbGenerator = require('generator-loopback'); | ||
| const yeoman = lbGenerator._yeoman; // generator-loopback should export _yeoman | ||
| assert(yeoman, 'generator-loopback should export _yeoman'); | ||
| const yeomanEnv = lbGenerator._yeomanEnv; // generator-loopback should export _yeomanEnv |
There was a problem hiding this comment.
nitpick: I don't think this comment is needed considering the assert on the next line
hacksparrow
left a comment
hacksparrow
left a comment
There was a problem hiding this comment.
With the Taranveer and Miroslav's points addressed, this LGTM.
6930ebf to
b8dc3b2
Compare
|
FYI, the jenkins machines seem don't install the latest Travis tests all pass. I would like to merge this PR then create another PR to fix the jenkins error. |
Perhaps @rmg would know better / how to remove it if npm used a cached version of the module. |
b8dc3b2 to
876a3d4
Compare
04dba18 to
96077af
Compare
5 participants
Description
The follow up PR of strongloop/generator-loopback#363, please make sure the
generator-loopbackPR get merged first and a new major version of it is released.Related issues
strongloop/generator-loopback#355
Checklist
guide